Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run tarpaulin against all packages in CI #1013

Merged
merged 1 commit into from
Apr 10, 2023
Merged

Conversation

changhc
Copy link
Contributor

@changhc changhc commented Apr 10, 2023

Part of #700.

Currently, tarpaulin only runs tests against onefetch, but when it calculates test coverage, all source files including other packages are taken into consideration, which is why some files like ascii/src/lib.rs has 0 coverage even if test cases do exist. To fix this, tarpaulin needs an additional parameter --workspace to run tests in all packages in this repo.

Before:

$ cargo tarpaulin
...
|| Tested/Total Lines:
|| ascii/src/lib.rs: 0/119
...
54.87% coverage, 670/1221 lines covered

After:

$ cargo tarpaulin --workspace
...
|| Tested/Total Lines:
|| ascii/src/lib.rs: 90/119 +75.63%
...
63.06% coverage, 770/1221 lines covered, +8.19% change in coverage

@@ -88,7 +88,7 @@ jobs:
version: latest

- name: Coverage with tarpaulin
run: cargo tarpaulin --out Lcov -- --test-threads 1
run: cargo tarpaulin --workspace --out Lcov -- --test-threads 1
Copy link
Owner

@o2sh o2sh Apr 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted! That's almost a 10% bump for free, thanks @changhc 👍

As a side note, I will probably get rid of --test-thread 1 and test if it sill works on the CI, I forgot why I added this in the first place 😅

@o2sh o2sh merged commit ed47222 into o2sh:main Apr 10, 2023
@changhc changhc deleted the fix-ci-tarpaulin branch April 10, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants